Skip to content

Add missing Stylelint properties and automatic grouping #6221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add missing Stylelint properties and automatic grouping #6221

wants to merge 8 commits into from

Conversation

@colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Sep 15, 2025 •
edited
Loading

Hope you don't mind us giving back from nhsuk/nhsuk-frontend#1589?

We spotted the Stylelint config order/properties-order list was getting a bit out of date

For example, it was missing:

  • Flex layout properties like flex and align-items
  • Column layout properties like column-gap and row-gap
  • Grid layout properties like grid-template-columns
  • Masking using mask-image, mask-repeat etc

Plus some readily used obscure properties such as pointer-events and text-decoration-thickness

This PR adds missing properties and turns on automatic grouping

Automatic grouping

Support for automatic grouping was added in [email protected] using emptyLineBefore: 'threshold'

Sass files in GOV.UK Frontend are manually now automatically split up into groups:

    box-sizing: border-box;
+ 
    display: inline-block;
+
    width: $indicator-size;
    height: $indicator-size;
    margin: 0 0.125em;
+
    border-width: 2px;
    border-style: solid;
    border-radius: 50%;

i.e. One less thing to fix manually when saving files

Format on save

Visual Studio code can be configured to automatically apply ESLint, Stylelint and Prettier formatting:

"editor.codeActionsOnSave": {
  "source.fixAll.eslint": "explicit",
  "source.fixAll.stylelint": "explicit"
},
"editor.defaultFormatter": "esbenp.prettier-vscode",
"prettier.requireConfig": true,
"stylelint.validate": ["css", "scss"],

@owenatgov
Copy link
Contributor

owenatgov commented Nov 18, 2025

@colinrotherham Thanks for this! I'm planning to get to this this week, if not today, so I've rebased it as a start

@owenatgov
Copy link
Contributor

owenatgov commented Nov 18, 2025

Not sure how those test failures got in as I reran this locally during rebase using the same command as the test 🤔 I'll fix them in a jiffy since it was my rebase that introduced them

@colinrotherham
Copy link
Contributor Author

colinrotherham commented Nov 18, 2025

@owenatgov I don't think the checks ran previously did they? This commit 6464237 caused it

I'd removed rules "unnecessary" for NHS.UK frontend but you still need them

Proves that it works 😆

@owenatgov
Copy link
Contributor

owenatgov commented Nov 18, 2025

Also @colinrotherham a littel update on this: The devs are happy with the added stylelint rules but we're less cohesive on the use of groups, specifically the spacing between groups. This is a preference thing between team members so for my money the answer is that we make a call on it. Gonna try to get this figured out soon as poss.

@colinrotherham
Copy link
Contributor Author

colinrotherham commented Nov 18, 2025

Also @colinrotherham a littel update on this: The devs are happy with the added stylelint rules but we're less cohesive on the use of groups, specifically the spacing between groups. This is a preference thing between team members so for my money the answer is that we make a call on it. Gonna try to get this figured out soon as poss.

Of course, that's the idea

We've been running with these settings on NHS.UK frontend for a little while and they've been a big help in answering "how would GOV.UK Frontend order this Sass"—including your usage of new lines too

But we've also seen that the spacing between some groups should be optional really

E.g. Keeping border-* and box-shadow together

So please don't take this PR "as is", make it work for you (if you want it)

@owenatgov
Copy link
Contributor

owenatgov commented Nov 18, 2025

We want it! 😂 We just need to work out what bits of it we want. Thanks for the added context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@colinrotherham @owenatgov